Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#218 enable cross fork compare #219

Merged
merged 1 commit into from
Dec 1, 2015
Merged

#218 enable cross fork compare #219

merged 1 commit into from
Dec 1, 2015

Conversation

if6was9
Copy link

@if6was9 if6was9 commented Sep 21, 2015

this addresses #218

@buildhive
Copy link

Kohsuke Kawaguchi » github-api #394 SUCCESS
This pull request looks good
(what's this?)

fqid1 = String.format("%s:%s",owner1,id1.getName());
fqid2 = String.format("%s:%s",owner2,id2.getName());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

catch should be after } -> } catch {

@lanwen
Copy link
Contributor

lanwen commented Sep 21, 2015

👎 For now.

I think it should be:

  1. Get the owners. (Can it be exception here? If not, skip 2 step)
  2. Use default owner names if exc.
  3. If owners equals, use getCompare(id1.getName(), id2.getName());
  4. Else format owner:branch and use getCompare with that

@buildhive
Copy link

Kohsuke Kawaguchi » github-api #395 SUCCESS
This pull request looks good
(what's this?)

@@ -682,7 +682,24 @@ public GHCompare getCompare(GHCommit id1, GHCommit id2) throws IOException {
}

public GHCompare getCompare(GHBranch id1, GHBranch id2) throws IOException {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please avoid empty lines in count more than one

@buildhive
Copy link

Kohsuke Kawaguchi » github-api #396 SUCCESS
This pull request looks good
(what's this?)

String ownerName1 = owner1.getOwnerName();
String ownerName2 = owner2.getOwnerName();
if (!StringUtils.equals(ownerName1, ownerName2)) {
String qualifiedName1 = String.format("%s:%s",ownerName1,id1.getName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add space after ,

@lanwen
Copy link
Contributor

lanwen commented Sep 27, 2015

👍 after some code style fixes

@buildhive
Copy link

Kohsuke Kawaguchi » github-api #397 SUCCESS
This pull request looks good
(what's this?)

@oleg-nenashev
Copy link
Collaborator

As I see from the code, null owners are not supposed to happen. BTW I'm not sure I've investigated all potential cases.

👍 in any case, because the original behavior smells like a bug

kohsuke added a commit that referenced this pull request Dec 1, 2015
@kohsuke kohsuke merged commit 723bb89 into hub4j:master Dec 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants